ostree-sign-ed25519: Fix overwriting already-set GErrors

There were a couple of issues here, related to passing a `GError**` into
a sub-function inside a loop:
 - If multiple calls inside the loop failed (e.g. if multiple keys
   inside the file were invalid, because it was a malformed file), the
   `GError` would be overwritten and cause a critical warning from GLib.
 - If an early iteration failed, but then subsequent ones succeeded, the
   `_load_pk_from_stream()` function would return `TRUE` but would also
   set its `GError`, which is a very non-standard (basically prohibited)
   thing to do with a `GError` argument.

Fix that by using a `first_error` variable internally and only
propagating it to the caller on failure.

This can be reproduced by trying to verify a signature using a key file
which isn’t actually a key file:
```
ostree sign --verify --repo ./my-repo $commit_id \
  --keys-file ./test.published.gpg
```

The keys file is expected to be in a custom format, with one ed25519 key
per line. An ASCII-armoured GPG keyring is not that format, and causes
it to go big boom.

Signed-off-by: Philip Withnall <pwithnall@gnome.org>

diff --git a/src/libostree/ostree-sign-ed25519.c b/src/libostree/ostree-sign-ed25519.c
index e51080986ae3af108e6f39993ecb35ff46196b78..22e0ee5a1350a8a7e037eb2d6e9b5e613c45a2a6 100644 (file)
--- a/src/libostree/ostree-sign-ed25519.c
+++ b/src/libostree/ostree-sign-ed25519.c
@@ -492,6 +492,8 @@ static gboolean
 _load_pk_from_stream (OstreeSign *self, GInputStream *key_stream_in, gboolean trusted,
                       GError **error)
 {
+  g_autoptr (GError) first_error = NULL;
+
   if (key_stream_in == NULL)
     return glnx_throw (error, "ed25519: unable to read from NULL key-data input stream");
 
@@ -514,16 +516,17 @@ _load_pk_from_stream (OstreeSign *self, GInputStream *key_stream_in, gboolean tr
           return FALSE;
         }
 
+      /* No more blobs? */
       if (blob == NULL)
-        return ret;
+        break;
 
       /* Read the key itself */
       pk = g_variant_new_from_bytes (G_VARIANT_TYPE_BYTESTRING, blob, FALSE);
 
       if (trusted)
-        added = ostree_sign_ed25519_add_pk (self, pk, error);
+        added = ostree_sign_ed25519_add_pk (self, pk, (first_error == NULL) ? &first_error : NULL);
       else
-        added = _ed25519_add_revoked (self, pk, error);
+        added = _ed25519_add_revoked (self, pk, (first_error == NULL) ? &first_error : NULL);
 
       g_autofree gchar *pk_printable = g_variant_print (pk, FALSE);
       g_debug ("%s %s key: %s", added ? "Added" : "Invalid", trusted ? "public" : "revoked",
@@ -534,6 +537,10 @@ _load_pk_from_stream (OstreeSign *self, GInputStream *key_stream_in, gboolean tr
         ret = TRUE;
     }
 
+  if (!ret)
+    g_propagate_prefixed_error (error, g_steal_pointer (&first_error),
+                                "ed25519: unable to read from NULL key-data input stream: ");
+
   return ret;
 }